bugfix(network): Reset network command id to keep commands in chronological order by Caball009 · Pull Request #2736 · TheSuperHackers/GeneralsGameCode

Caball009 · 2026-05-19T20:57:03Z

Synchronized network commands are given a unique id so that they can be sorted chronologically. The id needs to be unique per execution frame. It's a uint16 value so it'll overflow quite fast, especially because it's never reset and the starting value is 64000 by default.

When the network runahead decreases it's expected that multiple frames are executed right after each other. I noticed an issue with the overflow when I set the CRC interval to once per frame. When the overflow happened for a client combined with a decrease of the runahead, the order of the CRC messages was no longer guaranteed to be in chronological order for that execution frame. That would cause a mismatch if the overflow didn't happen for all clients at the same time.

With a starting value of 64000 and a CRC message every frame the first overflow happens after ~25 seconds. You can see it mismatch here because of the overflow (this was with a special test build);
https://www.youtube.com/live/V_l-q9Y-DmA?t=621s
https://www.youtube.com/live/V_l-q9Y-DmA?t=9499s

I've added some test code in the second commit. If that's used without this fix, it becomes very apparent that the messages are not in the right order when the command id overflows. ~On top of that, it's very likely that the disconnect bug (disconnect screen with everyone at ~~59XXX msec) occurs frequently.~~

I intend to do a follow up PR to make the static commandID variable a part of the NetCommandMsg class.

greptile-apps · 2026-05-31T18:51:04Z

Greptile Summary

This PR fixes a command-ID overflow bug in the network synchronization layer. The static commandID in GenerateNextCommandID() was a function-local uint16 starting at 64000 that was never reset, causing it to wrap around to 0 every ~1535 commands — and when the overflow didn't coincide across all clients it produced out-of-order CRC/FRAMEINFO messages and disconnect bugs.

NetworkUtil.cpp: Lifts commandID to a file-scope static (s_commandID) initialized to 0, adds GetCommandID() and ResetCommandID() helpers, and changes from pre-increment to post-increment semantics.
Network.cpp: Calls ResetCommandID() in init() for a clean start, and in update() resets the counter to 0 whenever it reaches 64000 and the network is transitioning to a new execution frame.
NetCommandList.cpp: Adds a comment documenting why overflow handling in the sort is intentionally deferred.

Confidence Score: 5/5

Safe to merge; the change is a targeted fix to a well-understood overflow bug with no regressions introduced.

The reset is placed correctly before GetCommandsFromCommandList() in every update cycle, the threshold of 64000 stays well below the uint16 maximum, and the init()-level reset ensures a clean start each game. The guard condition mirrors the exact predicate used by getExecutionFrame(), so the reset fires once per new execution frame on all deterministic clients simultaneously.

No files require special attention.

Important Files Changed

Filename	Overview
Core/GameEngine/Source/GameNetwork/NetworkUtil.cpp	Refactors the command-ID counter to file scope and adds GetCommandID()/ResetCommandID(); semantics change from pre- to post-increment is intentional and correct.
Core/GameEngine/Source/GameNetwork/Network.cpp	Adds reset in init() and a threshold-based reset in update() before command collection; guard condition correctly ensures reset only fires when entering a new execution frame.
Core/GameEngine/Include/GameNetwork/networkutil.h	Two new function declarations added cleanly; header already uses #pragma once.
Core/GameEngine/Source/GameNetwork/NetCommandList.cpp	Documentation-only change: adds a rationale comment explaining the known overflow limitation in addMessage() sorting.

Sequence Diagram

sequenceDiagram
    participant GL as GameLogic
    participant NET as Network::update()
    participant NU as NetworkUtil
    participant CCL as GetCommandsFromCommandList()
    participant NCL as NetCommandList

    GL->>NET: frame advances
    NET->>NU: GetCommandID()
    alt "commandID >= 64000 AND new execution frame"
        NET->>NU: ResetCommandID()
    end
    NET->>CCL: collect game messages
    CCL->>NU: GenerateNextCommandID()
    CCL->>NCL: addMessage(cmd)
    NCL-->>NCL: sort by commandID
    NCL-->>NET: ordered command list

_{Reviews (6): Last reviewed commit: "Changed implementation." | Re-trigger Greptile}

helmutbuhler · 2026-06-02T23:26:07Z

Resetting the command id just delays the potential mismatch, right? Wouldn't it be better to sort the command ids properly, even if that rarely causes mismatches with retail clients?

Caball009 · 2026-06-03T01:27:17Z

Resetting the command id just delays the potential mismatch, right?

It's reset every N frames, so it's not just delayed.

xezon

The id needs to be unique per execution frame.

Can we just reset it to 0 or 1 at the beginning of every new frame? Or does that cause issues? What is the reason for it to reset every 128 frames now?

xezon · 2026-06-03T18:33:30Z

-	static UnsignedShort commandID = 64000;
-	++commandID;
-	return commandID;
+static UnsignedShort commandID = 0;


Is it ok to start it at 0 when retail clients still start at 64000 ? I suspect this means every client can make up his own Id numbers as he pleases ?

Any clue why the original author picked 64000 as a start for it ?

Is it ok to start it at 0 when retail clients still start at 64000 ?

Yes, this is ok. It's an important argument in favor of resetting the command ID as opposed to fixing the sorting implementation. The latter is not retail-compatible and relatively complicated.

I suspect this means every client can make up his own Id numbers as he pleases ?

Yes, each client can use their own value.

Any clue why the original author picked 64000 as a start for it ?

Not sure. Perhaps they were testing overflow behavior and forgot to change it back. If that's correct, I suppose the question becomes why didn't they notice the issues that come with overflows. They didn't have multiple instances so they'd be more constrained in their ability to stress test it, but I'm only speculating.

xezon · 2026-06-03T18:43:09Z


+	const UnsignedInt frame = TheGameLogic->getFrame();
+	if (m_localStatus == NETLOCALSTATUS_INGAME) {
+		if (frame + m_runAhead > m_lastExecutionFrame) {


I do not understand this condition. Can you explain it?

It's extracted from Network::getExecutionFrame. It should prevent the command id from getting reset in the middle of an execution frame.

helmutbuhler · 2026-06-04T17:12:30Z

When it's reset every 128 frames, isn't it possible that a packet from frame 127 comes in after it's already reset and then gets sorted wrong?

xezon · 2026-06-05T08:02:07Z

My current understanding is that the ids only matter per frame, not across different frames. But then I do not understand why it needs to be reset every 128 frames instead of every frame.

This reverts commit 3685b17.

This reverts commit 5efd806.

Caball009 · 2026-06-07T19:02:12Z

I noticed that at extremely high latency (beyond 64 frames runahead) the disconnect screen could be triggered by the reset somehow. I tried a ton of things, but I was unable to find out why that happens. That's why I made some changes to the implementation and only reset the command id when it gets close to overflowing.

When it's reset every 128 frames, isn't it possible that a packet from frame 127 comes in after it's already reset and then gets sorted wrong?

if (frame + m_runAhead > m_lastExecutionFrame)

This check should guard against that. If the check is inverted, you'll see wrong sorting once in a while.

My current understanding is that the ids only matter per frame, not across different frames. But then I do not understand why it needs to be reset every 128 frames instead of every frame.

If the runahead decreases the ids matter across multiple frames.

Caball009 added Bug Something is not working right, typically is user facing Major Severity: Minor < Major < Critical < Blocker Network Anything related to network, servers Gen Relates to Generals ZH Relates to Zero Hour labels May 19, 2026

Caball009 force-pushed the fix_network_commandID_overflow branch 2 times, most recently from ee6bcd8 to 1f31061 Compare May 31, 2026 18:32

Caball009 marked this pull request as ready for review May 31, 2026 18:46

Caball009 requested a review from Mauller May 31, 2026 18:46

Caball009 force-pushed the fix_network_commandID_overflow branch from 1f31061 to ebf1fa2 Compare May 31, 2026 18:59

Mauller reviewed Jun 2, 2026

View reviewed changes

Comment thread Core/GameEngine/Source/GameNetwork/NetworkUtil.cpp Outdated

Caball009 force-pushed the fix_network_commandID_overflow branch from ebf1fa2 to a7ff356 Compare June 2, 2026 11:45

Caball009 force-pushed the fix_network_commandID_overflow branch from a7ff356 to 3ae2622 Compare June 3, 2026 01:29

xezon reviewed Jun 3, 2026

View reviewed changes

Caball009 force-pushed the fix_network_commandID_overflow branch from 3ae2622 to 93c5ce7 Compare June 4, 2026 17:07

Caball009 added 6 commits June 6, 2026 17:36

Added experimental overflow sorting.

3685b17

Revert "Added experimental overflow sorting."

e60c7b5

This reverts commit 3685b17.

Added test code.

5efd806

Revert "Added test code."

33f90dd

This reverts commit 5efd806.

Added code to reset command id.

5f2134d

Changed implementation.

1bccb3a

Caball009 force-pushed the fix_network_commandID_overflow branch from 93c5ce7 to 1bccb3a Compare June 7, 2026 18:54

Conversation

Caball009 commented May 19, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

greptile-apps Bot commented May 31, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Greptile Summary

Confidence Score: 5/5

Important Files Changed

Sequence Diagram

Uh oh!

Uh oh!

helmutbuhler commented Jun 2, 2026

Uh oh!

Caball009 commented Jun 3, 2026

Uh oh!

xezon left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

xezon Jun 3, 2026

Choose a reason for hiding this comment

Uh oh!

Caball009 Jun 4, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

xezon Jun 3, 2026

Choose a reason for hiding this comment

Uh oh!

Caball009 Jun 7, 2026

Choose a reason for hiding this comment

Uh oh!

helmutbuhler commented Jun 4, 2026

Uh oh!

xezon commented Jun 5, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Caball009 commented Jun 7, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

Caball009 commented May 19, 2026 •

edited

Loading

greptile-apps Bot commented May 31, 2026 •

edited

Loading

Caball009 Jun 4, 2026 •

edited

Loading

xezon commented Jun 5, 2026 •

edited

Loading